Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve highlights across languages #2910

Merged

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Jun 29, 2022

This is a party pack of small highlights improvements for:

  • erlang
  • markdown
  • bash
  • edoc
  • rust
  • html
  • heex
  • make
  • tsq (tree-sitter-query)
  • git-commit

Included is a fix for #2882. It's a bit different from the neovim version: we just switch scopes when the commit message gets too long - first to @warning and then @error.

The only change I think is controversial is 83d2c1e - people might prefer we always use @markup.raw.block as a background for code-fences even when we inject other languages. Let me know what you think!

The '#' character may either be interpreted as a map when used
like so:

    %% Example 1
    #{a => b}

Or as an operator which updates an existing map when the left-hand
side is an expression:

    %% Example 2
    MyMap#{a => b}

This commit changes the highlight to `punctuation.bracket` when used
as a character in a literal map (example 1) and keeps the `operator`
highlight when used for updating (example 2).
* add punctuation highlights for commas as in function parameters
* remove stray `variable.parameter` highlight
    * I couldn't find any regressions from this and it fixes an
      edge case I ran into (but sadly did not record 😓)
* highlight `fn` as `keyword.function`
    * the theme docs have `fn` as an example so it seems fitting
Punctuation highlights would show up outside of where they
were valid, for example using parentheses in some text. This
change prevents that by gating the captures to being under
the named nodes in which they are valid.
* `/>` as in self-closing tags like `<hr/>`
* `=` as in the separator between attribute name and value `<a href="bar">`
`module` is undocumented and does not exist in other themes. The
equivalent existing scope based on usage (Elixir for example) is
`namespace`.
This includes a fix for the new HTML highlights introduced a few
parent commits back:

    ["\"" (attribute_name)] @string

Would get tripped up and the entire line would be highlighted as
a string. Now `\"` is a valid escape.

I'm switching to my fork as the primary repo as the upstream hasn't
been touched in over a year (mostly because stability afaict) but
it has no watchers currently so I'm not hopeful that my PR will
be merged.
@the-mikedavis the-mikedavis force-pushed the md-highlight-improvement-party-pack branch from e000547 to 7ff9c8d Compare June 29, 2022 17:48
@poliorcetics
Copy link
Contributor

About e2b8506, I don't agree at all with this. Personally I limit my commit messages length to 80 or 100, never to 50 characters and being told this in an error by the editor will make it so helix is forcing git oudated (to me) recommendations. I can understand other peoples wanting it but this should be configurable, not absolute

@archseer
Copy link
Member

Also, isn't configuring a ruler enough? It won't color your text when you go over, but it should still work all the same.

to 80 or 100, never to 50 characters

Well, it's 50 for the heading, then the body is traditionally formatted to 80 to fit on terminal screens.

people might prefer we always use @markup.raw.block as a background for code-fences even when we inject other languages.

Yeah, I think we should keep that scope? That way backgrounds can be styled. Why was it removed, did you have clashing styling?

@poliorcetics
Copy link
Contributor

Well, it's 50 for the heading, then the body is traditionally formatted to 80 to fit on terminal screens.

I'm speaking for the heading too

@kirawi
Copy link
Member

kirawi commented Jun 30, 2022

It can be configured by changing the query.

@the-mikedavis
Copy link
Member Author

Hmm yeah it doesn't feel great to hard-code that behavior into the query. Ideally once we have a better config language we can make it a configurable feature (enabled/disabled plus the widths to use). But also a ruler does feel like the appropriate tool, at least for now. I'll leave a comment on #2882 and drop that out of this PR.

That way backgrounds can be styled. Why was it removed, did you have clashing styling?

Oh I didn't think to style markup.raw.block only with bg 😅. Yeah I'll drop that change. I didn't like un-highlighted text like shell code-fences for example inheriting the fg of markup.raw.block but just using bg instead solves that.

git clone [email protected]:helix-editor/helix.git
# <- function
#     ^ inherited
#          ^ inherited

* branch message with current branch and diverged branch has been
  added to the parser
* scissors used in verbose commits are marked as a punctuation
  delimiter
    * we could use comment instead since they're visually the
      same but IMO this works better
You might use a macro like `?MODULE` to name a record:

    -record(?MODULE, {a, b, c}).

With this fix, the record fields correctly get `variable.other.member`
highlights.
@the-mikedavis the-mikedavis force-pushed the md-highlight-improvement-party-pack branch from 7ff9c8d to c916ceb Compare June 30, 2022 22:36
@archseer archseer merged commit c5600c9 into helix-editor:master Jul 1, 2022
@the-mikedavis the-mikedavis deleted the md-highlight-improvement-party-pack branch July 1, 2022 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants